Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lifetime-free range methods #732

Merged
merged 2 commits into from
Jan 20, 2024
Merged

Add lifetime-free range methods #732

merged 2 commits into from
Jan 20, 2024

Conversation

cberner
Copy link
Owner

@cberner cberner commented Dec 21, 2023

Fixes #725

@cberner cberner force-pushed the arc branch 2 times, most recently from 20142b5 to 8677438 Compare December 22, 2023 03:34
@Frando
Copy link

Frando commented Jan 13, 2024

Hi, this is great! In iroh we have to return owned iterators to tables, and thus far resorted to the ouroboros crate, which allows to create self-referential structs through very unsafe means. We'd love to get rid of that, because ouroboros was plagued with unsoundness issues in the past.

However one of our owned iterators contains a Range to table B, but returns values from ReadOnlyTable A (table B is an index table, and table A contains the actual values). To remove the need for ouroboros here, redb would have to provide an ArcReadOnlyTable in addition to the ArcRange I think. From reading the code, this seems feasable to add, in the same way as the ArcRange works. What do you think?

@cberner
Copy link
Owner Author

cberner commented Jan 15, 2024

I think I'll be able to just remove all the lifetimes from ReadOnlyTable. I haven't gotten to prototyping it yet, but one of my goals for 2.0 is to only have lifetimes on objects used for writing

@jeromegn
Copy link

jeromegn commented Jan 15, 2024

I've seen more crates have functions and structs use the owned or Owned prefix/suffix. Maybe there's a future where there's no Arc usage in there and suddenly the naming makes less sense when using that as a prefix :)

one of my goals for 2.0 is to only have lifetimes on objects used for writing

Is there a limitation requiring a lifetime for writes? I assume that means WriteTransaction and Table would still require a lifetime bound.

My current experiment is making a DBMS backed by redb, using rhai as its "query language". rhai requires static lifetimes for custom types. My temporary solution is to use a once_cell::sync::Lazy to get a 'static redb database. I do have to open tables very often because they also have lifetimes. I can't keep them around as custom objects passed via rhai. Anyway, all this to say that "owned" types would help a lot for these kinds of things!

@cberner
Copy link
Owner Author

cberner commented Jan 15, 2024

Is there a limitation requiring a lifetime for writes? I assume that means WriteTransaction and Table would still require a lifetime bound.

I think the lifetime can be removed from WriteTransaction, and maybe Table too. The Range returned from Table has to have a lifetime though, because it effectively borrows from Table which is mutable

@cberner cberner marked this pull request as draft January 16, 2024 01:23
@cberner cberner changed the title Add range_arc() and get_arc() Add lifetime-free range methods Jan 20, 2024
@cberner cberner marked this pull request as ready for review January 20, 2024 18:09
These methods return reference counted iterators from read only
transactions
@cberner cberner merged commit a6acb67 into master Jan 20, 2024
3 checks passed
@cberner cberner deleted the arc branch January 20, 2024 19:26
@cberner
Copy link
Owner Author

cberner commented Jan 20, 2024

@Frando I think the latest master will allow you remove that. read-only tables are now returned with the 'static lifetime

@Frando
Copy link

Frando commented Feb 27, 2024

@cberner I'm struggling to create an owned Iterator for a range. Do you have an example somewhere? I was on the latest release from crates.io, with the git version it all works

@cberner
Copy link
Owner Author

cberner commented Feb 28, 2024

Great! Ya, it's not released yet because I want to get a couple more breaking changes in before I release 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add read API without lifetimes
3 participants